[pull] master from git:master#116
Merged
pull[bot] merged 16 commits intoturkdevops:masterfrom Oct 20, 2025
Merged
Conversation
If one of the two provided paths for git diff --no-index ends in a '/', a failure similar to the following occurs: $ git diff --no-index -- /tmp/ /tmp/ ':!' fatal: `pos + len' is too far after the end of the buffer This occurs because of an incorrect calculation of the skip lengths in diff_no_index(). The code wants to calculate the length of the string, but add one in case the string doesn't end with a slash. The method it uses is incorrect, as it always checks the trailing NUL character of the string. This will never be a '/', so we always add one. In the event that we *do* have a trailing slash, this will create an off-by-one length error later when using the skip value. The most straightforward fix would be to correct the skip1 and skip2 lengths by using ends_with(). However, Johannes made a good point that the existing logic is wasting a lot of computation. We generate the match string by copying the path in and then skipping almost all of it immediately with a potentially expensive memmove() from the strbuf_remove() call. We also re-initialize the match stringbuf each time we call read_directory_contents. The read_directory_contents really wants a path that is rooted at the start of the directory scan. We're currently building this by taking the full path and stripping out the start portion. Instead, replace this logic by building up the portion of the match as we go. Start by initializing two strbuf in diff_no_index containing the empty string. Pass these into queue_diff, which in turn passes the appropriate left or right side into read_directory_contents. As before, we build up the matches by appending elements to the match path and then clearing them using strbuf_setlen. In the recursive portion of the queue_diff algorithm, we build up new match paths the same way that we build up new buffer paths, by appending the elements and then clearing them with strbuf_setlen after each iteration. This is cheaper as it avoids repeated allocations, and is a bit simpler to track what is going on. Add a couple of test cases that pass in paths already ending in '/', to ensure the tests cover this regression. Reported-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Closes: https://lore.kernel.org/git/c75ec5f9-407a-6555-d4fb-bb629d54ec61@gmx.de/ Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> [jc: small leakfixes at the end of diff_no_index()] Signed-off-by: Junio C Hamano <gitster@pobox.com>
In a future patch series the 'xdiff' Rust crate will be added. Delete the creation of the static library file for xdiff to avoid a name conflict. This also moves toward the goal of Rust only needing to link against libgit.a. Changes to Meson are not required as the xdiff library is already included in Meson's libgit.a. xdiff-objs was a historical make target to allow building just the objects in xdiff. Since it was defined in terms of XDIFF_OBJS (which no longer exists) this convenience make target no longer makes sense. Remove it. Signed-off-by: Ezekiel Newren <ezekielnewren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Same idea as the previous commit except that I don't know when or if reftable will be turned into a Rust crate. Signed-off-by: Ezekiel Newren <ezekielnewren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The options j, J, k, and K don't affect the status of the current hunk. They just go to a different one. This is true whether the current hunk is undecided or not. Avoid misunderstanding by no longer mentioning the current hunk explicitly in their help texts. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The variable "permitted" is not reset after moving to a different hunk, so it only accumulates permission and doesn't necessarily reflect those of the current hunk. This may be a bug, but is actually useful with the option J, which can be used at the last hunk to roll over to the first hunk. Make this particular behavior official. Also adjust the error message, as it will only be shown if there's just a single hunk. Suggested-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The options y, n, and e mark the current hunk as decided. If there's another undecided hunk towards the bottom of the hunk array they go there. If there isn't, but there is another undecided hunk towards the top then they go to the very first hunk, no matter if it has already been decided on. The option j does basically the same move. Technically it is not allowed if there's no undecided hunk towards the bottom, but the variable "permitted" is never reset, so this permission is retained from the very first hunk. That may a bug, but this behavior is at least consistent with y, n, and e and arguably more useful than refusing to move. Improve the roll-over behavior of these four options by moving to the first undecided hunk instead of hunk 1, consistent with what they do when not rolling over. Also adjust the error message for j, as it will only be shown if there's no other undecided hunk in either direction. Reported-by: Windl, Ulrich <u.windl@ukr.de> Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Options j and J roll over at the bottom and go to the first undecided hunk and hunk 1, respectively. Let options k and K do the same when they reach the top of the hunk array, so let them go to the last undecided hunk and the last hunk, respectively, for consistency. Also use the same direction-neutral error messages. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Options a and d stage and unstage all undecided hunks towards the bottom of the array of hunks, respectively, and then roll over to the very first hunk. The first part is similar to y and n if the current hunk is the last one in the array, but they roll over to the next undecided hunk if there is any. That's more useful; do it for a and d as well. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Don't accumulate allowed options from any visited hunks, start fresh at the top of the loop instead and only record the allowed options for the current hunk. Reported-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In d255105 (SubmittingPatches: release-notes entry experiment, 2024-03-25), we began an experiment to have contributors suggest a topic description to appear in our RelNotes and "What's cooking?" reports. Extend that experiment to also welcome suggested topic branch names in addition to descriptions. Suggested-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Occasionally there are efforts to contribute to the Git project that span more than one patch series in order to achieve a broader goal. By convention, the maintainer has typically suffixed the topic names with "-part-one", or "-part-1" and so on. Document that convention and suggest some guidance on how to structure proposed topic names for multi-series efforts. Suggested-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Instead of three library archives (one for git, one for reftable, and one for xdiff), roll everything into a single libgit.a archive. This would help later effort to FFI into Rust. * en/make-libgit-a: make: delete REFTABLE_LIB, add reftable to LIB_OBJS make: delete XDIFF_LIB, add xdiff to LIB_OBJS
The code in "git add -p" and friends to iterate over hunks was riddled with bugs, which has been corrected. * rs/add-patch-options-fix: add-patch: reset "permitted" at loop start add-patch: let options a and d roll over like y and n add-patch: let options k and K roll over like j and J add-patch: let options y, n, j, and e roll over to next undecided add-patch: document that option J rolls over add-patch: improve help for options j, J, k, and K
A few more things that patch authors can do to help maintainer to keep track of their topics better. * tb/doc-submitting-patches: SubmittingPatches: guidance for multi-series efforts SubmittingPatches: extend release-notes experiment to topic names
An earlier addition to "git diff --no-index A B" to limit the output with pathspec after the two directories misbehaved when these directories were given with a trailing slash, which has been corrected. * jk/diff-no-index-with-pathspec-fix: diff --no-index: fix logic for paths ending in '/'
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )